-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: add MissingDataError #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Documentation build overview
Show files changed (4 files in total): 📝 4 modified | ➕ 0 added | ➖ 0 deleted
|
forgot about NODEID
should work this time!
introducing the utility function get_node_columns_separate
agitter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to break the get_node_columns signature? Adding the checks within get_node_* functions makes sense to me, as does the new error type in general. My understanding is that the scope argument exists only so we can pass it through to the error message. That is more readable, but adds overhead in using these functions. The alternative would be to use the stack trace to see which algorithm generated the error.
Those are my initial thoughts before a deep review.
While this only affects actual runtime code with an updated, consistent error message on malformed data, the goal of
MissingDataErroris to create a finer distinction between actual unexpected SPRAS errors and expected one, to further #21 (comment). Closes #393.This breaks the
get_node_columnssignature, requiring updates in other open algorithm PRs.